fix(dev-preview): a running relay owns 8080 — a stale watch pid no longer turns it into "held by another process" - #2575
Merged
Conversation
…y otherwise
Production, the first re-pick: the relay restarted under a new pid (4698);
the realtime ports/watch snapshot still carried the previous relay's pid
(11264), because the channel never reported the re-bind. describeHttpPortSlot
compared the two, called the relay a stranger, and the pane read "Port 8080
is held by another process in the sandbox (pid 11264)" while the planner
refused the pick with http-port-busy — against a healthy relay.
A `running` relay bound 8080 or it would have exited (`server.on('error')`
→ `process.exit(1)`) and stopped being running; so a listener beside a live
relay IS the relay, whatever a watch pid says. Only a fresh `probe` pid that
differs may name a user process. `HttpPortSlotInput.listenerSource` carries
that, threaded through the status, the planner and describeServiceState.
Tests: the stale-watch-pid case (relay), the same disagreement from a probe
(user process), planner and state siblings; the two existing assertions
that expected refusal on a watch pid now say `probe`. Mutation-checked.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8A8KLYCoViP7ouH5QzbnB
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe preview logic now treats listener PIDs from watch snapshots as potentially stale when a relay is live. Authoritative probe PIDs still distinguish relay ownership from user processes. Service planning, status reporting, and tests now propagate and validate the listener source. ChangesPreview slot classification
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Preview port ownership now tolerates stale watch PIDs for a live relay while fresh probe conflicts remain blocked, avoiding false blocked states without weakening confirmed port-conflict handling. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What you saw
What was actually true (read from the sandbox)
ss -ltnpon the env sprite: 8080 is held by our own relay (node -e … 8080 3000, pid 4698,running, started 20:45 by the re-pick);:3000isnext-server. Pid 11264 no longer exists — it was the previous relay incarnation (created 17:48 by the first pick). The realtimeports/watchsnapshot still carried pid 11264 because the channel never reported the re-bind.describeHttpPortSlotcompared the snapshot's pid with the relay's, saw a mismatch, and called the relay a stranger — so the status readblocked,canOpenwas false, and the planner refused the pick withhttp-port-busy.Fix
A
runningrelay bound 8080 or it would have exited (server.on('error') → process.exit(1)) and stopped being running. So a listener beside a live relay is the relay, whatever awatchpid says; only a freshprobepid that differs may name a user process.HttpPortSlotInputgainslistenerSource(default'watch'), threaded throughbuildDevPreviewStatus, the planner'shttp-port-busycheck anddescribeServiceState. The select path already re-probes and plans with'probe', so a genuine stranger on 8080 is still refused there.Realtime's fold already replaces a same-port entry on
port_opened; the channel just never sent one — no realtime change.Tests
Stale watch pid →
relay; same disagreement from a probe →user-process; planner does not refuse on a watch pid but does on a probe pid;describeServiceStateisliveon a watch pid,blockedon a probe pid. The two existing assertions that expected refusal on any pid mismatch now sayprobe. Mutation-checked: removing the source check fails exactly the new assertions.After deploy
The pane for the pinned
:3000shows Live and the frame renders (given #2572 is live too). No re-pick needed.🤖 Generated with Claude Code
https://claude.ai/code/session_01P8A8KLYCoViP7ouH5QzbnB
Summary by CodeRabbit